Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The beginning of grabbing Sonarr root folders and approving requests with sepecific root folders #535

Closed
wants to merge 2 commits into from

Conversation

onedr0p
Copy link
Contributor

@onedr0p onedr0p commented Sep 13, 2016

Here's a brief rundown of what I did and what is left to do.

In the Admin Setting for Sonarr:

  • Removed Folder Path textbox
  • Added button to grab root folders
  • Added drop down with Root folders
  • The code will send an API request to Sonarr and grab the folders when you hit the 'Get Root Folders' button https://github.com/Sonarr/Sonarr/wiki/Rootfolder
  • Updated some logic thru-out the program to accept a rootFolderId (like how you handled qualityId)

What's left to do:

  • Figure out why when you save it won't update the cache with the root folders grabbed.
  • It won't populate the root folders from cache when you save and reload the Admin Settings > Sonarr
  • Implement Approving requests with selecting a root folder.

@@ -186,6 +186,37 @@ private Negotiator LoadRequests()

}

IEnumerable<RootFolderModel> rootFolders = new List<RootFolderModel>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This module is not being used yet just a FYI, there is no code being used in this class

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured as much, I just copied what I wrote in the RequestModule.cs :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@onedr0p Ok that's fine :)

@tidusjar
Copy link
Member

Looks good so far!

So the caching is not working for you?

@onedr0p
Copy link
Contributor Author

onedr0p commented Sep 14, 2016

Nope, I hit the button to grab the root folders, click save and reload the page and they are wiped out. I am clueless as to why :/

Also after I added the validation stuff in SonarrSettingsValidator it won't accept you even hitting Save. Sorry my C# is very bad lol

@tidusjar
Copy link
Member

Hey that's fine. I can take a look tonight to see if I can find out what is going wrong.

Also after I added the validation stuff in SonarrSettingsValidator it won't accept you even hitting Save
What wouldn't accept? Not sure I understand.

Your C# is not bad! You've been able to read and understand what is going on and there is some confusing areas so that's execllent!

@onedr0p
Copy link
Contributor Author

onedr0p commented Sep 14, 2016

I have a strong Linux background, and I recently started a position as a VB.net web developer and I slowly getting up to par with the Microsoft environment. Thanks for taking a look!

@tidusjar
Copy link
Member

OK, so just looked at your code, you are storing the root folders into the cache (Default cache expiry is 20 minutes). When we save the sonarr settings Here we are binding the results from the <form> being submitted and binding it to the model, in this case SonarrSettings.

Now SonarrSettings does not contain a definition for the root folders, so they are not being saved into the database.
So when reloading the page, we query the database for the SonarrSettings and the root folders do not exist.

So what we need to do is add a new property onto the SonarrSettings something like a string[] to store those root folders (You already have it in the cache, so just get it from the cache and assign it to the new string[] property 👍 ).

If you need any help please ask 😄

data: $form.serialize(),
url: "sonarrrootfolders",
dataType: "json",
success: function(response) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a bug here where if we keep pressing the Get RootFolders it will keep appending to the list :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that lol, it's not an important fix for right now thou

@@ -80,12 +80,24 @@
</div>

<div class="form-group">
<div>
<button type="submit" id="getRootFolders" class="btn btn-primary-outline">Get RootFolders <div id="getRootFolderSpinner" /></button>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we include a space in RootFolders so it's Root Folders ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that makes sense :)

Copy link
Member

@tidusjar tidusjar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments

@onedr0p
Copy link
Contributor Author

onedr0p commented Sep 14, 2016

Now SonarrSettings does not contain a definition for the root folders, so they are not being saved into the database.

In teh SonarrSettings class I have

        public bool Enabled { get; set; }
        public string ApiKey { get; set; }
        public string QualityProfile { get; set; }
        public string RootFolder { get; set; }
        public string RootPath { get; set; }
        public bool SeasonFolders { get; set; }

The array of paths will also be stored here with the one the user selected? Or should I change it to:

        public bool Enabled { get; set; }
        public string ApiKey { get; set; }
        public string QualityProfile { get; set; }
        public string[] RootFolder { get; set; }
        // public string RootPath { get; set; }
        public bool SeasonFolders { get; set; }

@onedr0p
Copy link
Contributor Author

onedr0p commented Sep 14, 2016

Using those new github features I see :P

@tidusjar
Copy link
Member

Well you need to know the one the user selected right?

Ignore my other comment, when saving we need to get it out of the cache and set the one the user picked into the RootFolders property.

On the approve page we can just request the root folder from Sonarr again.

@Magikarplvl4
Copy link
Contributor

@onedr0p is the pull ready for merge?

@tidusjar
Copy link
Member

It's not finished yet. I think I'll merge it in and then finish it off :)

@Magikarplvl4 Magikarplvl4 added this to the Release v2.0 milestone Nov 3, 2016
@tidusjar tidusjar changed the title The beggining of grabbing Sonarr root folders and approving requests with sepecific root folders The beginning of grabbing Sonarr root folders and approving requests with sepecific root folders Nov 16, 2016
@Magikarplvl4 Magikarplvl4 mentioned this pull request Dec 21, 2016
@Magikarplvl4 Magikarplvl4 modified the milestones: Release v2.0, V2.1 release Jan 4, 2017
@onedr0p
Copy link
Contributor Author

onedr0p commented Jan 12, 2017

Sorry for the late response, please continue where I left off.

@Magikarplvl4 Magikarplvl4 modified the milestones: 2.2, V2.X release pool Jan 12, 2017
@tidusjar tidusjar closed this Jan 23, 2017
tidusjar pushed a commit that referenced this pull request Jan 23, 2017
@Magikarplvl4 Magikarplvl4 modified the milestones: Release v2.1, Release v2.2 Jan 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants